Adding CRT and PDS info in calib ntuple by sungbinoh · Pull Request #422 · SBNSoftware/sbndcode

sungbinoh · 2024-02-29T02:42:12Z

Notes

reco2_sce.fcl is updated to include results from additional analysis modules (CRTCalibAna and SBNDFlashAssAna) into the calibration ntuple.
CRTCalibAna is based on Henry's CRTAnalysis module and ICARUS's calibration ntuple structure. CRTCalibAna has Henry's tree in it for just in case. If we are sure about variable selections for CRT in the calibration nutple, we might select them and remove Henry's tree from the calibration ntuple.
SBNDFlashAssAna is exactly same with ICARUS's analysis module (https://github.com/SBNSoftware/icaruscode/blob/develop/icaruscode/PMT/OpReco/ICARUSFlashAssAna_module.cc) that adds PMT information into calibration ntuple.

Discussions

I added "PDS" directory for SBNDFlashAssAna module. If someone wants to rearrange it, please feel free to do so. (For Linyan and Mike, calibration ntuples that I produced using Grid to test the workflow still have "PMT" for variable names..)
Modified "PMT" into "PDS" inside SBNDFlashAssAna where the original file from ICARUS has PMT since PMTs are only light detector of the ICARUS detector.
I added several Iines in reco2_sce.fcl forCRTCalibAna and SBNDFlashAssAna . If someone wants to include those lines in reco_sbnd.fcl, please feel free to do so.
The TriggerLabel: "generator" in reco2_sce.fcl did not work during my test for workflow since there was no trigger information in the codes that are compatible with 2023B. If we have them for the new production, please modify this line with correct TriggerLabel name.

…sce.fcl and additional modules.

VCLanNguyen · 2024-02-29T17:42:32Z

Hi @sungbinoh, I think it's better to declare all the producer and analyzer module in reco_sbnd.fcl
Example here:
https://github.com/SBNSoftware/sbndcode/blob/develop/sbndcode/JobConfigurations/base/reco_sbnd.fcl#L244-L250

And then inherit the module in the reco2 fcl.
With this setup, both reco2_sce and non-sce can inherit the new modules.
Example here:
https://github.com/SBNSoftware/sbndcode/blob/develop/sbndcode/JobConfigurations/standard/reco/reco2_sce.fcl

(I know editing reco_sbnd.fcl is a bit of a pain because it's quite messy at the moment 😞 )

The fcl files are updated according to the suggestion. I tested with a small sample and everything still seems to work. Could you please have a look? Thanks Lan.

Hi @linyan-w , I add some more comments mostly for tidying up. Otherwise looks good to me.

VCLanNguyen · 2024-02-29T17:58:05Z

On point 4:

I don't think SBND workflow currently simulates trigger information so there's no trigger info in the code at all.
Therefore, I don't know what trigger products SBND will use in the future, it might or might not be the following two products since SBND does not share the same hardware trigger as ICARUS.

#include "lardataobj/RawData/TriggerData.h"
#include "sbnobj/Common/Trigger/ExtraTriggerInfo.h"

fjnicolas · 2024-03-01T17:00:11Z

@sungbinoh there are already some PDS related directories (OpDetSim, OpDetReco and OpDetAnalyzer), I'd probably move the PDS analyzer to OpDetAnalyzer

linyan-w · 2024-03-11T14:15:47Z

@sungbinoh there are already some PDS related directories (OpDetSim, OpDetReco and OpDetAnalyzer), I'd probably move the PDS analyzer to OpDetAnalyzer

The PDS analyzer in this version is dedicated to calibration, and will be further trimmed down in later development. That's different from the PDS analyzer and we'd like to keep them separate, just like the TPC analyzer and the TPC calibration part is separate.

fjnicolas · 2024-03-11T14:34:36Z

@sungbinoh there are already some PDS related directories (OpDetSim, OpDetReco and OpDetAnalyzer), I'd probably move the PDS analyzer to OpDetAnalyzer

The PDS analyzer in this version is dedicated to calibration, and will be further trimmed down in later development. That's different from the PDS analyzer and we'd like to keep them separate, just like the TPC analyzer and the TPC calibration part is separate.

@linyan-w Can we have then a common parent directory (e.g. Calibration) for all the calibration related analyzers? We can make then subdirectories for each subsystem (e.g. CRT, PDS...) with all these dedicated analyzers devoted exclusively to detector calibration. As it is right now, we have CRT calibration related analyzers in CRT/CRTAna and PDS ones in PDS/OpReco (and that's definitely not PDS reconstruction, but analysis)

NB: there is currently a Calibration directory containing code for the 1D signal processing (so the naming is not very accurate here). If the calibration WG want to take over this directory we can just move all the 1D signal processing stuff to some other directory with a better naming

linyan-w · 2024-03-11T14:45:33Z

@sungbinoh there are already some PDS related directories (OpDetSim, OpDetReco and OpDetAnalyzer), I'd probably move the PDS analyzer to OpDetAnalyzer

The PDS analyzer in this version is dedicated to calibration, and will be further trimmed down in later development. That's different from the PDS analyzer and we'd like to keep them separate, just like the TPC analyzer and the TPC calibration part is separate.

@linyan-w Can we have then a common parent directory (e.g. Calibration) for all the calibration related analyzers? We can make then subdirectories for each subsystem (e.g. CRT, PDS...) with all these dedicated analyzers devoted exclusively to detector calibration. As it is right now, we have CRT calibration related analyzers in CRT/CRTAna and PDS ones in PDS/OpReco (and that's definitely not PDS reconstruction, but analysis)

That's a good idea. I'm not sure we want to do so in the non-empty Calibration directory. Let me have a look at the existing modules there.

…com/SBNSoftware/sbndcode into feature/sungbinoh_crt_pds_for_calib

fjnicolas · 2024-03-12T16:01:45Z

@linyan-w the Calibration directory is now empy, in case you want to move the analyzers/skimmer fhicl configurations there. If not for this PR, can we at least move the SBNDFlashAssAna to the OpDetAnalyzer directory so we avoid having duplicated/unnecesarry directories for the light detection system code? Thank you!

fjnicolas · 2024-03-12T17:37:10Z

@VCLanNguyen happy with the last changes?

VCLanNguyen · 2024-03-12T17:45:43Z

@VCLanNguyen happy with the last changes?

@linyan-w misses deleting some lines regarding services in reco2_sce.fcl and move detector clock service from reco2_sce.fcl to reco_sbnd.fcl.
Should be a quick edit then good to go.

linyan-w · 2024-03-12T18:01:39Z

@VCLanNguyen happy with the last changes?

@linyan-w misses deleting some lines regarding services in reco2_sce.fcl and move detector clock service from reco2_sce.fcl to reco_sbnd.fcl. Should be a quick edit then good to go.

How about now?

VCLanNguyen · 2024-03-12T18:09:30Z

@VCLanNguyen happy with the last changes?

@linyan-w misses deleting some lines regarding services in reco2_sce.fcl and move detector clock service from reco2_sce.fcl to reco_sbnd.fcl. Should be a quick edit then good to go.

How about now?

Can you completely delete the three lines in reco2_sce.fcl about services . Effectively there should be no changes in the reco2_sce.fcl

Co-authored-by: VCLanNguyen <74775793+VCLanNguyen@users.noreply.github.com>

VCLanNguyen · 2024-03-12T18:21:59Z

All look good to me.

fjnicolas · 2024-03-12T18:22:38Z

thank you @sungbinoh @linyan-w !

fjnicolas · 2024-03-12T18:22:44Z

trigger build LArSoft/lar*@LARSOFT_SUITE_v09_85_00

VCLanNguyen · 2024-03-12T18:23:04Z

On point 4:

* I don't think SBND workflow currently simulates trigger information so there's no trigger info in the code at all.

* Therefore, I don't know what trigger products SBND will use in the future, it might or might not be the following two products since SBND does not share the same hardware trigger as ICARUS.

#include "lardataobj/RawData/TriggerData.h"
#include "sbnobj/Common/Trigger/ExtraTriggerInfo.h"

@linyan-w I would open an issue about the pmt skim module does not work with trigger product right now since it's not available on SBND. Just so we can keep track of it in the future.

linyan-w · 2024-03-12T18:35:36Z

thank you @sungbinoh @linyan-w !

Thanks for the help Lan and Fran!

linyan-w · 2024-03-12T18:37:00Z

On point 4:
* I don't think SBND workflow currently simulates trigger information so there's no trigger info in the code at all.

* Therefore, I don't know what trigger products SBND will use in the future, it might or might not be the following two products since SBND does not share the same hardware trigger as ICARUS.
#include "lardataobj/RawData/TriggerData.h"
#include "sbnobj/Common/Trigger/ExtraTriggerInfo.h"
@linyan-w I would open an issue about the pmt skim module does not work with trigger product right now since it's not available on SBND. Just so we can keep track of it in the future.

We have multiple items for improvement on this ntuple as we deferred several suggestions and found several issues already. I currently have these in a slack chat with Sungbin. If the nominal workflow is to document them in issues, then sure I can open one.

VCLanNguyen · 2024-03-12T18:45:19Z

On point 4:
* I don't think SBND workflow currently simulates trigger information so there's no trigger info in the code at all.

* Therefore, I don't know what trigger products SBND will use in the future, it might or might not be the following two products since SBND does not share the same hardware trigger as ICARUS.
#include "lardataobj/RawData/TriggerData.h"
#include "sbnobj/Common/Trigger/ExtraTriggerInfo.h"
@linyan-w I would open an issue about the pmt skim module does not work with trigger product right now since it's not available on SBND. Just so we can keep track of it in the future.
We have multiple items for improvement on this ntuple as we deferred several suggestions and found several issues already. I currently have these in a slack chat with Sungbin. If the nominal workflow is to document them in issues, then sure I can open one.

I think opening an Issue + Improvement would be good for tracking and good practice! Thanks!

FNALbuild · 2024-03-12T18:50:46Z

✔️ CI build for LArSoft Succeeded on slf7 for c14:prof -- details available through the CI dashboard

FNALbuild · 2024-03-12T18:55:45Z

✔️ CI build for LArSoft Succeeded on slf7 for e26:prof -- details available through the CI dashboard

linyan-w · 2024-03-12T18:57:52Z

On point 4:
* I don't think SBND workflow currently simulates trigger information so there's no trigger info in the code at all.

* Therefore, I don't know what trigger products SBND will use in the future, it might or might not be the following two products since SBND does not share the same hardware trigger as ICARUS.
#include "lardataobj/RawData/TriggerData.h"
#include "sbnobj/Common/Trigger/ExtraTriggerInfo.h"
@linyan-w I would open an issue about the pmt skim module does not work with trigger product right now since it's not available on SBND. Just so we can keep track of it in the future.
We have multiple items for improvement on this ntuple as we deferred several suggestions and found several issues already. I currently have these in a slack chat with Sungbin. If the nominal workflow is to document them in issues, then sure I can open one.
I think opening an Issue + Improvement would be good for tracking and good practice! Thanks!

OK, I'm opening an issue here:
#427
Will assign it to Sungbin and others (hopefully I can find someone in PDS/CRT groups who can help with the trimming).

FNALbuild · 2024-03-12T19:11:26Z

❌ CI build for SBND Failed at phase unit_test SBND on slf7 for c14:prof - ignored warnings for build -- details available through the CI dashboard

🚨 For more details about the failed phase, check the unit_test SBND phase logs

parent CI build details are available through the CI dashboard

FNALbuild · 2024-03-12T19:37:57Z

⚠️ CI build for SBND Warning at phase ci_tests SBND on slf7 for e26:prof - ignored warnings for build -- details available through the CI dashboard

🚨 For more details about the warning phase, check the ci_tests SBND phase logs

parent CI build details are available through the CI dashboard

fjnicolas · 2024-03-13T09:17:15Z

On point 4:
* I don't think SBND workflow currently simulates trigger information so there's no trigger info in the code at all.

* Therefore, I don't know what trigger products SBND will use in the future, it might or might not be the following two products since SBND does not share the same hardware trigger as ICARUS.
#include "lardataobj/RawData/TriggerData.h"
#include "sbnobj/Common/Trigger/ExtraTriggerInfo.h"
@linyan-w I would open an issue about the pmt skim module does not work with trigger product right now since it's not available on SBND. Just so we can keep track of it in the future.
We have multiple items for improvement on this ntuple as we deferred several suggestions and found several issues already. I currently have these in a slack chat with Sungbin. If the nominal workflow is to document them in issues, then sure I can open one.

Yes, opnening a issue is the best practice, thanks @linyan-w! All the CI tests passed, PR ready to merge in the next release.

A commit to add CRT and PDS info in calib ntuple using updated reco2_…

b6c0926

…sce.fcl and additional modules.

VCLanNguyen reviewed Feb 29, 2024

View reviewed changes

Merge branch 'develop' into feature/sungbinoh_crt_pds_for_calib

234018d

linyan-w added 5 commits March 11, 2024 15:54

Re-organize the fcl files for calibration skimmers

c4c238b

Re-organize the calibration skimmer fcls

e1e23ad

Bug fix for the last commit...

41f0e90

Merge branch 'feature/sungbinoh_crt_pds_for_calib' of https://github.…

c3af2ef

…com/SBNSoftware/sbndcode into feature/sungbinoh_crt_pds_for_calib

Add comments to explain the CRT variables

1ffea3b

VCLanNguyen reviewed Mar 12, 2024

View reviewed changes

Comment thread sbndcode/JobConfigurations/standard/reco/reco2_sce.fcl Outdated

Comment thread sbndcode/JobConfigurations/standard/reco/reco2_sce.fcl Outdated

Comment thread sbndcode/JobConfigurations/base/reco_sbnd.fcl Outdated

Comment thread sbndcode/JobConfigurations/base/reco_sbnd.fcl Outdated

VCLanNguyen reviewed Mar 12, 2024

View reviewed changes

Comment thread sbndcode/JobConfigurations/standard/reco/reco2_sce.fcl Outdated

Remove some comments to tidy up

c2876f2

linyan-w approved these changes Mar 12, 2024

View reviewed changes

Merge branch 'develop' into feature/sungbinoh_crt_pds_for_calib

5429cc2

linyan-w added 4 commits March 12, 2024 11:21

Move PDS (calibration) to Calibration

e2c6a16

Re-organize CRT fcl

cb68545

Re-organize CRT fcl

2bf4e75

Move crt config out of reco_sbnd

c03f524

Clock etc

0249bcd

VCLanNguyen closed this Mar 12, 2024

VCLanNguyen reopened this Mar 12, 2024

VCLanNguyen requested changes Mar 12, 2024

View reviewed changes

Comment thread sbndcode/JobConfigurations/standard/reco/reco2_sce.fcl Outdated

Comment thread sbndcode/JobConfigurations/standard/reco/reco2_sce.fcl Outdated

linyan-w and others added 2 commits March 12, 2024 13:17

Update sbndcode/JobConfigurations/standard/reco/reco2_sce.fcl

da5e56d

Co-authored-by: VCLanNguyen <74775793+VCLanNguyen@users.noreply.github.com>

Update sbndcode/JobConfigurations/standard/reco/reco2_sce.fcl

990167f

Co-authored-by: VCLanNguyen <74775793+VCLanNguyen@users.noreply.github.com>

fjnicolas merged commit 4f12ea8 into develop Mar 22, 2024

Uh oh!

Conversation

sungbinoh commented Feb 29, 2024

Uh oh!

VCLanNguyen Feb 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

linyan-w Mar 12, 2024

Choose a reason for hiding this comment

Uh oh!

VCLanNguyen Mar 12, 2024

Choose a reason for hiding this comment

Uh oh!

VCLanNguyen commented Feb 29, 2024

Uh oh!

fjnicolas commented Mar 1, 2024

Uh oh!

linyan-w commented Mar 11, 2024

Uh oh!

fjnicolas commented Mar 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

linyan-w commented Mar 11, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fjnicolas commented Mar 12, 2024

Uh oh!

fjnicolas commented Mar 12, 2024

Uh oh!

VCLanNguyen commented Mar 12, 2024

Uh oh!

linyan-w commented Mar 12, 2024

Uh oh!

VCLanNguyen commented Mar 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

VCLanNguyen commented Mar 12, 2024

Uh oh!

fjnicolas commented Mar 12, 2024

Uh oh!

fjnicolas commented Mar 12, 2024

Uh oh!

VCLanNguyen commented Mar 12, 2024

Uh oh!

linyan-w commented Mar 12, 2024

Uh oh!

linyan-w commented Mar 12, 2024

Uh oh!

VCLanNguyen commented Mar 12, 2024

Uh oh!

FNALbuild commented Mar 12, 2024

Uh oh!

FNALbuild commented Mar 12, 2024

Uh oh!

linyan-w commented Mar 12, 2024

Uh oh!

FNALbuild commented Mar 12, 2024

Uh oh!

FNALbuild commented Mar 12, 2024

Uh oh!

fjnicolas commented Mar 13, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

VCLanNguyen Feb 29, 2024 •

edited

Loading

fjnicolas commented Mar 11, 2024 •

edited

Loading

VCLanNguyen commented Mar 12, 2024 •

edited

Loading